SITCOM-1042: Add initial python code #1
Conversation
fred3m
changed the title
fred3m
force-pushed
the
tickets/SITCOM-1042
branch
from
d7c241c to
6e537a0
Compare
fred3m
force-pushed
the
tickets/SITCOM-1042
branch
2 times, most recently
from
9a3513d to
0db1749
Compare
fred3m
force-pushed
the
tickets/SITCOM-1042
branch
from
17f0bc6 to
2464942
Compare
|
|
||
| def on_error(ws: WebSocketApp, error: str) -> None: | ||
| """Error received from the server.""" | ||
| print(f"\033[91mError: {error}\033[0m") |
There was a problem hiding this comment.
in mock_server.py there is a nicer way to show console colors (at least more redeable). Maybe we can add a printc function?
class Color(Enum):
RED = 0x33
printc(message, color)
There was a problem hiding this comment.
Ok, I implemented this in a new rubinTV_analysis_service.utils module. I didn't do this before because the mock server (and even the print functions in the client module) aren't going to be used in production, they're just debugging tools as we develop the package, but I guess it doesn't hurt to include aprintc method.
|
|
||
|
|
||
| # Registry of all commands | ||
| command_registry = {} |
There was a problem hiding this comment.
.- Move it as static attribute of the class BaseCommand
Or maybe:
.- add as static field on a new class where we can also add the execute_command (i.e. CommandHandler)
Another possibility is that it may be here on purpose....
There was a problem hiding this comment.
This had been intentional, but I like your method better. I didn't realize that I could attach a static variable to BaseCommand and just use the register class method to add commands to the same shared registry (but of course now that I think about it this behavior makes sense).
|
|
||
| def __call__(self, table: sqlalchemy.Table) -> sqlalchemy.sql.elements.BooleanClauseList: | ||
| child_results = [child(table) for child in self.children] | ||
| try: |
There was a problem hiding this comment.
I don't mind converting this to match/case for readability, but using a dictionary lookup for 4 cases that won't ever change seems like overkill.
No description provided.